Skip to content

feat(ISV-7220): switch pipelines to rh-direct-sign-image#2320

Draft
Allda wants to merge 2 commits into
konflux-ci:developmentfrom
Allda:ISV-7219-ISV-7920
Draft

feat(ISV-7220): switch pipelines to rh-direct-sign-image#2320
Allda wants to merge 2 commits into
konflux-ci:developmentfrom
Allda:ISV-7219-ISV-7920

Conversation

@Allda

@Allda Allda commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Replaces rh-sign-image with rh-direct-sign-image across the three managed release pipelines:

  • rh-advisories
  • rh-push-to-registry-redhat-io
  • rh-push-helm-chart-to-registry-redhat-io

The new task calls the signing script directly as a command (it is now on PATH) instead of going through RADAS. The releasePlanAdmissionPath parameter, which
is not used by rh-direct-sign-image, has been removed from all three pipeline task invocations.

References in integration test assertions, READMEs, and task description prose have been updated to match the new task name.

This PR depends on ISV-7115 which introduces the rh-direct-sign-image task.

JIRA: ISV-7220, ISV-7219

Test plan

  • rh-advisories-large-snapshot integration test passes with rh-direct-sign-image
  • rh-advisories-idempotent integration test: second release correctly skips rh-direct-sign-image
  • rh-push-to-registry-redhat-io pipeline signs images successfully
  • rh-push-helm-chart-to-registry-redhat-io pipeline signs images successfully

Relevant Jira

Checklist before requesting a review

  • I have marked as draft or added do not merge label if there's a dependency PR
    • If you want reviews on your draft PR, you can add reviewers or add the release-service-maintainers handle if you are unsure who to tag
  • My commit message includes Signed-off-by: My name <email>
  • I read CONTRIBUTING.MD and commit formatting
  • I have run the README.md generator script in .github/scripts/readme_generator.sh and verified the results using .github/scripts/check_readme.sh
  • If an AI agent was used, I marked that via a commit footer like Assisted-By: Cursor

@Allda Allda marked this pull request as draft June 24, 2026 10:26
@qodo-app-for-konflux-ci

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The mock file is named with a .py extension but uses a Bash shebang and Bash syntax. This can be confusing and may break if the task runner expects a real Python script based on filename/entrypoint conventions. Consider renaming to a .sh mock or converting it to an actual Python script.

#!/usr/bin/env bash
set -eux
echo "Mock rh_direct_sign_image.py called with: $*"
echo "$*" >> "${DATA_DIR}/mock_rh_direct_sign_image.txt"
Type Mismatch

Some params are declared as type: string but use unquoted numeric defaults (e.g., concurrentLimit). Tekton/YAML parsing may treat these as integers, which can lead to inconsistent behavior when values are interpolated as strings. Consider quoting numeric defaults for string-typed params.

- name: requestTimeout
  type: string
  default: "1800"
  description: InternalRequest timeout
- name: concurrentLimit
  type: string
  description: The maximum number of signing requests to run in parallel
  default: 8
- name: pipelineRunUid
  type: string
📚 Focus areas based on broader codebase context

Missing Guard

The new rh-direct-sign-image pipeline task is only guarded by skip_release, but other signing-related tasks also guard on $(tasks.collect-task-params.results.extractedValues[1]) being non-empty. Consider adding the same when predicate so rh-direct-sign-image is skipped when there are no signing parameters / no signing work to do. (Ref 1, Ref 2)

- name: rh-direct-sign-image
  timeout: "6h00m0s"
  when:
    - input: "$(tasks.filter-already-released-advisory-images.results.skip_release)"
      operator: in
      values: ["false"]
  retries: 3

Reference reasoning: In the existing managed pipelines, signing tasks like rh-sign-image-cosign include a when check ensuring a key extracted value is not empty before running. Aligning rh-direct-sign-image with this established gating pattern reduces unnecessary TaskRuns and matches how the pipeline currently avoids running signing logic when inputs are absent.

📄 References
  1. konflux-ci/release-service-catalog/pipelines/managed/rh-advisories/rh-advisories.yaml [517-534]
  2. konflux-ci/release-service-catalog/pipelines/managed/rh-push-to-registry-redhat-io/rh-push-to-registry-redhat-io.yaml [328-342]
  3. konflux-ci/release-service-catalog/pipelines/managed/rh-push-helm-chart-to-registry-redhat-io/rh-push-helm-chart-to-registry-redhat-io.yaml [357-371]
  4. konflux-ci/release-service-catalog/pipelines/managed/rh-advisories/rh-advisories.yaml [876-882]
  5. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/rh-sign-image.yaml [1-11]
  6. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/tests/test-rh-sign-image-single-component-plr-alreadysigned.yaml [140-164]
  7. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/tests/test-rh-sign-image-single-component-plr.yaml [140-164]
  8. konflux-ci/release-service-catalog/tasks/managed/rh-sign-image/tests/test-rh-sign-image-single-component.yaml [140-164]

@qodo-app-for-konflux-ci

qodo-app-for-konflux-ci Bot commented Jun 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (5) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 28 rules

Grey Divider


Action required

1. Mock script missing pipefail 📘 Rule violation ☼ Reliability
Description
The newly added mock script uses set -eux and does not enable pipefail, violating the
strict-mode requirement for standalone shell scripts. Missing pipefail can hide failures in piped
commands during tests.
Code

tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[R1-2]

+#!/usr/bin/env bash
+set -eux
Relevance

⭐⭐ Medium

Strict-mode/pipefail suggestions appear in reviews but outcomes are mostly undetermined; mixed
enforcement signal (e.g., PR #2100).

PR-#2100

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1261 requires standalone scripts to enable strict error handling (set -euo pipefail or
equivalent). The script explicitly sets set -eux, omitting pipefail.

Rule 1261: Shell scripts must enable strict error handling flags
tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[1-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The standalone bash script does not enable required strict-mode flags (`-euo pipefail`).

## Issue Context
This file is an executable mock used by tests; strict mode reduces flakiness and prevents silent failures.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[1-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Tekton test script uses exit 1 📘 Rule violation ≡ Correctness
Description
The Tekton script: in the test pipeline exits non-zero (exit 1) to signal failures instead of
always exiting 0 and reporting status via Tekton results. This can violate task conventions expected
by the platform and makes status handling inconsistent.
Code

tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[R130-151]

+              mock_file="$(params.dataDir)/mock_rh_direct_sign_image.txt"
+              if [ ! -f "${mock_file}" ]; then
+                echo "rh_direct_sign_image was not called"
+                exit 1
+              fi
+
+              args=$(cat "${mock_file}")
+
+              if ! echo "${args}" | grep -q -- "--requester testuser"; then
+                echo "Expected --requester testuser in args: ${args}"
+                exit 1
+              fi
+
+              if ! echo "${args}" | grep -q -- "--pyxis-server production"; then
+                echo "Expected --pyxis-server production in args: ${args}"
+                exit 1
+              fi
+
+              if ! echo "${args}" | grep -q -- "--submit-requests"; then
+                echo "Expected --submit-requests in args: ${args}"
+                exit 1
+              fi
Relevance

⭐⭐ Medium

Some reviews question exit 1 conventions (PR #2191), but no consistent accepted precedent found.

PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1256 requires Tekton task scripts to always exit 0 and use results for status. The added script
contains multiple exit 1 branches in the new test pipeline TaskSpec step.

Rule 1256: Tekton task scripts must always exit 0 and use results for status
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[126-151]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A Tekton taskSpec `script:` exits with `exit 1` on validation failures.

## Issue Context
Per the compliance requirement, Tekton task scripts should not use process exit codes for business logic; they should write a success/failure value to a result and exit 0.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[126-153]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Unquoted command substitution in args 📘 Rule violation ≡ Correctness
Description
The script assigns args=$(cat ...) without quoting the command substitution, which violates the
requirement to quote command substitutions. This can lead to surprising behavior if the substituted
content contains special characters.
Code

tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[136]

+              args=$(cat "${mock_file}")
Relevance

⭐ Low

Team has rejected similar shell-quoting strictness suggestions in tests/mocks before (PR #1861);
likely not enforced.

PR-#1861

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1263 requires quoting command substitutions as "$(command)". The script uses `args=$(cat
"${mock_file}")` without quotes around the substitution.

Rule 1263: Always quote shell variables and command substitutions
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[136-136]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The bash command substitution in a variable assignment is not quoted.

## Issue Context
The compliance requirement expects `"$(...)"` to be used instead of bare `$(...)`.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[136-136]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. publish-pyxis-repository/README.md edited 📘 Rule violation ⚙ Maintainability
Description
This PR adds and modifies README.md files under tasks/managed/ (specifically
tasks/managed/rh-direct-sign-image/README.md and
tasks/managed/publish-pyxis-repository/README.md), but READMEs under tasks/ and pipelines/ are
treated as auto-generated and must not be added or edited in change sets. Editing these generated
docs can break generator-driven documentation and CI checks that enforce README immutability or
conflict with the README generation workflow.
Code

tasks/managed/publish-pyxis-repository/README.md[24]

+for the signing tasks (and `rh-direct-sign-image` runs quite early to begin with). So this means
Relevance

⭐ Low

Repo frequently updates tasks/managed READMEs via generator (PRs #1161, #1002); README edits are
normal here.

PR-#1161
PR-#1002

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 1260 prohibits any additions or modifications of README.md files under tasks/ or
pipelines/. The cited diffs show that this PR both changes content in
tasks/managed/publish-pyxis-repository/README.md at the referenced line and introduces a new
tasks/managed/rh-direct-sign-image/README.md across the referenced line range, directly violating
the rule’s restriction on generated README artifacts.

Rule 1260: Do not manually edit auto-generated README files under tasks/ and pipelines/
tasks/managed/publish-pyxis-repository/README.md[23-26]
tasks/managed/rh-direct-sign-image/README.md[1-33]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This PR adds and modifies `README.md` files under `tasks/` (including `tasks/managed/rh-direct-sign-image/README.md` and `tasks/managed/publish-pyxis-repository/README.md`), but these READMEs are considered auto-generated outputs and must not be introduced or edited directly in PR diffs.

## Issue Context
This repository treats READMEs under `tasks/`/`pipelines/` as generator-produced artifacts; updates should be made through the generator inputs/tools and workflow rather than by committing changes to generated README files, otherwise CI checks and the centralized README generation process may be broken or bypassed.

## Fix Focus Areas
- tasks/managed/publish-pyxis-repository/README.md[24-24]
- tasks/managed/rh-direct-sign-image/README.md[1-33]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Steps use computeResources not resources 📘 Rule violation ☼ Reliability
Description
The new Tekton Task specifies per-step resources using computeResources, but the compliance
requirement expects a resources block with both requests and limits for cpu/memory. This may
cause policy checks or tooling expecting resources to fail.
Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[R153-184]

+    - name: use-trusted-artifact
+      computeResources:
+        limits:
+          memory: 64Mi
+          cpu: 100m
+        requests:
+          memory: 64Mi
+          cpu: 30m
+      ref:
+        resolver: "git"
+        params:
+          - name: url
+            value: $(params.taskGitUrl)
+          - name: revision
+            value: $(params.taskGitRevision)
+          - name: pathInRepo
+            value: stepactions/use-trusted-artifact/use-trusted-artifact.yaml
+      params:
+        - name: workDir
+          value: $(params.dataDir)
+        - name: sourceDataArtifact
+          value: $(params.sourceDataArtifact)
+
+    - name: sign-image
+      image: quay.io/konflux-ci/release-service-utils@sha256:8ac7a9b0bb623775eca570edef8a0fc34f693f50a7e0a50547a4e496875d68b9
+      computeResources:
+        limits:
+          memory: 4Gi
+          cpu: "2"
+        requests:
+          memory: 4Gi
+          cpu: "2"
Relevance

⭐ Low

This repo standardizes on Tekton computeResources for step requests/limits (e.g., PR #1002); not
resources.

PR-#1002

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1265 requires each Task step to define a resources block with cpu/memory requests and limits
(or via stepTemplate.resources). The new Task steps define computeResources instead of
resources, and there is no stepTemplate.resources present.

Rule 1265: Specify compute resources in all Tekton tasks
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[152-184]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tekton step resource constraints are defined under `computeResources`, but the compliance requirement calls for `resources` with cpu/memory `requests` and `limits`.

## Issue Context
If your Tekton/API version supports only `computeResources`, document/adjust the policy; otherwise, rename/migrate to `resources` to satisfy enforcement.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[152-184]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Previous review results

Review updated until commit 6140581

Results up to commit 0c8e3ac


🐞 Bugs (0) 📘 Rule violations (5) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Mock script missing pipefail 📘 Rule violation ☼ Reliability
Description
The newly added mock script uses set -eux and does not enable pipefail, violating the
strict-mode requirement for standalone shell scripts. Missing pipefail can hide failures in piped
commands during tests.
Code

tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[R1-2]

+#!/usr/bin/env bash
+set -eux
Relevance

⭐⭐ Medium

Strict-mode/pipefail suggestions appear in reviews but outcomes are mostly undetermined; mixed
enforcement signal (e.g., PR #2100).

PR-#2100

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1261 requires standalone scripts to enable strict error handling (set -euo pipefail or
equivalent). The script explicitly sets set -eux, omitting pipefail.

Rule 1261: Shell scripts must enable strict error handling flags
tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[1-4]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The standalone bash script does not enable required strict-mode flags (`-euo pipefail`).

## Issue Context
This file is an executable mock used by tests; strict mode reduces flakiness and prevents silent failures.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[1-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
2. Tekton test script uses exit 1 📘 Rule violation ≡ Correctness
Description
The Tekton script: in the test pipeline exits non-zero (exit 1) to signal failures instead of
always exiting 0 and reporting status via Tekton results. This can violate task conventions expected
by the platform and makes status handling inconsistent.
Code

tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[R130-151]

+              mock_file="$(params.dataDir)/mock_rh_direct_sign_image.txt"
+              if [ ! -f "${mock_file}" ]; then
+                echo "rh_direct_sign_image was not called"
+                exit 1
+              fi
+
+              args=$(cat "${mock_file}")
+
+              if ! echo "${args}" | grep -q -- "--requester testuser"; then
+                echo "Expected --requester testuser in args: ${args}"
+                exit 1
+              fi
+
+              if ! echo "${args}" | grep -q -- "--pyxis-server production"; then
+                echo "Expected --pyxis-server production in args: ${args}"
+                exit 1
+              fi
+
+              if ! echo "${args}" | grep -q -- "--submit-requests"; then
+                echo "Expected --submit-requests in args: ${args}"
+                exit 1
+              fi
Relevance

⭐⭐ Medium

Some reviews question exit 1 conventions (PR #2191), but no consistent accepted precedent found.

PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1256 requires Tekton task scripts to always exit 0 and use results for status. The added script
contains multiple exit 1 branches in the new test pipeline TaskSpec step.

Rule 1256: Tekton task scripts must always exit 0 and use results for status
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[126-151]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A Tekton taskSpec `script:` exits with `exit 1` on validation failures.

## Issue Context
Per the compliance requirement, Tekton task scripts should not use process exit codes for business logic; they should write a success/failure value to a result and exit 0.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[126-153]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational
3. publish-pyxis-repository/README.md edited 📘 Rule violation ⚙ Maintainability
Description
This PR adds and modifies README.md files under tasks/managed/ (specifically
tasks/managed/rh-direct-sign-image/README.md and
tasks/managed/publish-pyxis-repository/README.md), but READMEs under tasks/ and pipelines/ are
treated as auto-generated and must not be added or edited in change sets. Editing these generated
docs can break generator-driven documentation and CI checks that enforce README immutability or
conflict with the README generation workflow.
Code

tasks/managed/publish-pyxis-repository/README.md[24]

+for the signing tasks (and `rh-direct-sign-image` runs quite early to begin with). So this means
Relevance

⭐ Low

Repo frequently updates tasks/managed READMEs via generator (PRs #1161, #1002); README edits are
normal here.

PR-#1161
PR-#1002

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 1260 prohibits any additions or modifications of README.md files under tasks/ or
pipelines/. The cited diffs show that this PR both changes content in
tasks/managed/publish-pyxis-repository/README.md at the referenced line and introduces a new
tasks/managed/rh-direct-sign-image/README.md across the referenced line range, directly violating
the rule’s restriction on generated README artifacts.

Rule 1260: Do not manually edit auto-generated README files under tasks/ and pipelines/
tasks/managed/publish-pyxis-repository/README.md[23-26]
tasks/managed/rh-direct-sign-image/README.md[1-33]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
This PR adds and modifies `README.md` files under `tasks/` (including `tasks/managed/rh-direct-sign-image/README.md` and `tasks/managed/publish-pyxis-repository/README.md`), but these READMEs are considered auto-generated outputs and must not be introduced or edited directly in PR diffs.

## Issue Context
This repository treats READMEs under `tasks/`/`pipelines/` as generator-produced artifacts; updates should be made through the generator inputs/tools and workflow rather than by committing changes to generated README files, otherwise CI checks and the centralized README generation process may be broken or bypassed.

## Fix Focus Areas
- tasks/managed/publish-pyxis-repository/README.md[24-24]
- tasks/managed/rh-direct-sign-image/README.md[1-33]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Unquoted command substitution in args 📘 Rule violation ≡ Correctness
Description
The script assigns args=$(cat ...) without quoting the command substitution, which violates the
requirement to quote command substitutions. This can lead to surprising behavior if the substituted
content contains special characters.
Code

tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[136]

+              args=$(cat "${mock_file}")
Relevance

⭐ Low

Team has rejected similar shell-quoting strictness suggestions in tests/mocks before (PR #1861);
likely not enforced.

PR-#1861

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1263 requires quoting command substitutions as "$(command)". The script uses `args=$(cat
"${mock_file}")` without quotes around the substitution.

Rule 1263: Always quote shell variables and command substitutions
tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[136-136]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The bash command substitution in a variable assignment is not quoted.

## Issue Context
The compliance requirement expects `"$(...)"` to be used instead of bare `$(...)`.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/test-rh-direct-sign-image.yaml[136-136]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Steps use computeResources not resources 📘 Rule violation ☼ Reliability
Description
The new Tekton Task specifies per-step resources using computeResources, but the compliance
requirement expects a resources block with both requests and limits for cpu/memory. This may
cause policy checks or tooling expecting resources to fail.
Code

tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[R153-184]

+    - name: use-trusted-artifact
+      computeResources:
+        limits:
+          memory: 64Mi
+          cpu: 100m
+        requests:
+          memory: 64Mi
+          cpu: 30m
+      ref:
+        resolver: "git"
+        params:
+          - name: url
+            value: $(params.taskGitUrl)
+          - name: revision
+            value: $(params.taskGitRevision)
+          - name: pathInRepo
+            value: stepactions/use-trusted-artifact/use-trusted-artifact.yaml
+      params:
+        - name: workDir
+          value: $(params.dataDir)
+        - name: sourceDataArtifact
+          value: $(params.sourceDataArtifact)
+
+    - name: sign-image
+      image: quay.io/konflux-ci/release-service-utils@sha256:8ac7a9b0bb623775eca570edef8a0fc34f693f50a7e0a50547a4e496875d68b9
+      computeResources:
+        limits:
+          memory: 4Gi
+          cpu: "2"
+        requests:
+          memory: 4Gi
+          cpu: "2"
Relevance

⭐ Low

This repo standardizes on Tekton computeResources for step requests/limits (e.g., PR #1002); not
resources.

PR-#1002

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1265 requires each Task step to define a resources block with cpu/memory requests and limits
(or via stepTemplate.resources). The new Task steps define computeResources instead of
resources, and there is no stepTemplate.resources present.

Rule 1265: Specify compute resources in all Tekton tasks
tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[152-184]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Tekton step resource constraints are defined under `computeResources`, but the compliance requirement calls for `resources` with cpu/memory `requests` and `limits`.

## Issue Context
If your Tekton/API version supports only `computeResources`, document/adjust the policy; otherwise, rename/migrate to `resources` to satisfy enforcement.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/rh-direct-sign-image.yaml[152-184]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Qodo Logo

Comment on lines +1 to +2
#!/usr/bin/env bash
set -eux

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Mock script missing pipefail 📘 Rule violation ☼ Reliability

The newly added mock script uses set -eux and does not enable pipefail, violating the
strict-mode requirement for standalone shell scripts. Missing pipefail can hide failures in piped
commands during tests.
Agent Prompt
## Issue description
The standalone bash script does not enable required strict-mode flags (`-euo pipefail`).

## Issue Context
This file is an executable mock used by tests; strict mode reduces flakiness and prevents silent failures.

## Fix Focus Areas
- tasks/managed/rh-direct-sign-image/tests/mocks/rh_direct_sign_image.py[1-4]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Allda and others added 2 commits July 2, 2026 10:23
New task for signing images using the direct signing method, which will
replace rh-sign-image that goes through Radas.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ales Raszka <araszka@redhat.com>
Replace rh-sign-image with rh-direct-sign-image in rh-advisories,
rh-push-to-registry-redhat-io, and
rh-push-helm-chart-to-registry-redhat-io pipelines. Remove the
releasePlanAdmissionPath param not used by the new task and update
references in tests, READMEs, and task description.

JIRA: ISV-7220, ISV-7219

Signed-off-by: Ales Raszka <araszka@redhat.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@Allda Allda force-pushed the ISV-7219-ISV-7920 branch from 0c8e3ac to 6140581 Compare July 2, 2026 08:38
@Allda Allda marked this pull request as ready for review July 2, 2026 11:27
@Allda Allda marked this pull request as draft July 2, 2026 11:30
@qodo-app-for-konflux-ci

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

The mock file is named with a .py extension but contains a Bash script shebang and Bash syntax. This can break execution in environments that dispatch based on file extension or expect Python behavior for .py files; consider renaming to a .sh-like name or converting it to an actual Python mock consistent with the task command.

#!/usr/bin/env bash
set -eux
echo "Mock rh_direct_sign_image.py called with: $*"
echo "$*" >> "${DATA_DIR}/mock_rh_direct_sign_image.txt"
Test Fragility

The yq patch targets .spec.steps[1] to inject DATA_DIR, which is brittle if step ordering changes (e.g., adding/reordering steps). Consider selecting the step by name (e.g., sign-image) to make the hook resilient to future edits.

# Inject DATA_DIR env var into the sign-image step so the mock binary can use it
# without relying on Tekton parameter substitution inside the mock script itself
yq -i '.spec.steps[1].env += [{"name": "DATA_DIR", "value": "$(params.dataDir)"}]' "$1"
Type Mismatch

concurrentLimit is declared as type: string but has a numeric default (8). Tekton parameter defaults should match the declared type; use "8" or change the param type to an integer (if supported/desired) to avoid validation or substitution surprises.

- name: concurrentLimit
  type: string
  description: The maximum number of signing requests to run in parallel
  default: 8
- name: pipelineRunUid

@qodo-app-for-konflux-ci

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 6140581

@Allda

Allda commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

/retest

1 similar comment
@Allda

Allda commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

/retest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants